Skip to content

JSON-RPC to graphQL migration#63

Merged
harshaalphafi merged 4 commits intomainfrom
feature/json-rpc-migration
May 2, 2026
Merged

JSON-RPC to graphQL migration#63
harshaalphafi merged 4 commits intomainfrom
feature/json-rpc-migration

Conversation

@11felix
Copy link
Copy Markdown
Contributor

@11felix 11felix commented Apr 30, 2026

No description provided.

@11felix 11felix requested a review from jangid as a code owner April 30, 2026 18:55
@11felix 11felix requested a review from Zorag44 April 30, 2026 18:56
Comment thread src/strategies/singleAssetLooping.ts Outdated
Comment thread src/models/blockchain.ts
Zorag44
Zorag44 previously approved these changes May 1, 2026
jangid
jangid previously approved these changes May 2, 2026
Copy link
Copy Markdown
Contributor

@jangid jangid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean migration from JSON-RPC to GraphQL-first initialization. Centralizing AlphalendClient into StrategyContext and removing the hardcoded 'mainnet' instances is a solid improvement. The missing await fix on updatePrices in singleAssetLooping.ts is a good catch too.

A few minor items to consider (non-blocking):

  1. alphaVault.ts:claimAirdrop still creates a local AlphalendClient instead of using this.context.alphalendClient like every other strategy
  2. alphaVault.ts ~line 1017 still hardcodes getConstants('mainnet') — should probably be getConstants(this.context.blockchain.network)
  3. types.ts — the JSDoc on graphqlUrl still says "Sui blockchain client for network operations" (carried over from suiClient)

@OctoSauce
Copy link
Copy Markdown
Contributor

scripts/testRun.ts still uses old SuiClient

OctoSauce
OctoSauce previously approved these changes May 2, 2026
Copy link
Copy Markdown
Contributor

@OctoSauce OctoSauce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few Nits. Rest LGTM

Comment thread src/core/index.ts
@11felix 11felix dismissed stale reviews from OctoSauce, rishikesh-chavan, jangid, and Zorag44 via 98a2fbb May 2, 2026 06:33
@rishikesh-chavan rishikesh-chavan requested review from Zorag44 and rishikesh-chavan and removed request for Zorag44 May 2, 2026 07:18
Copy link
Copy Markdown
Contributor

@jangid jangid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — fixes look good. Minor remaining items (unused txBuildClient? option in BlockchainOptions, scripts/testRun.ts still on old API, stale doc in index.ts) are non-blocking.

@harshaalphafi harshaalphafi merged commit bdc93ca into main May 2, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants